-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add dereferenceUses
plugin
#1279
base: main
Are you sure you want to change the base?
Conversation
I'm trying to run this using
however, the resultant SVG still has all its
|
Github won't let me attach the SVG file itself, so here's the full content as code block:
|
@Pomax: Oh right, sorry, that I hadn't explicitly mentioned this, but the plugin is not enabled by default (as this may not always be wished for default optimization) and has to be explicitly enabled when using
|
Right, but that's what I used? I issued |
@Pomax: Right, so |
Maybe I'm doing something wrong here, but I'm not seeing it replace the the I then tried both |
@Pomax: Alright, the reason was that the |
aha! let me give it another go =) |
Side note: All Note that the current version of the |
Sweet, that's working quite well now. It grows the file, of course, but the improvement in inspectability and (with --pretty) readability is night and day. Awesome work! |
@Pomax: Please also notice the side note above! It is possible to further unwrap these elements (from their |
I do notice that all the nested (specifically, this files -and about 250 files like it- is the result of running a |
@Pomax: It may be negligible for the effective SVG file size when dereferencing the |
fair point: once gzip is involved there is no size difference. |
@Pomax: There is the inlineStyles plugin, and as kind of opposite to that, another "externalizeStyles" plugin was planned, that could try to move all inline styles to external styles and simplify, but with GZIP it may be too much of a hassle. |
yeah definitely. Plus, for what I need, externalizing styles is not really an option, the files need to stay self-contained. If gzip/brotli makes it a non-optimization over the wire, then no reason to try to micro-optimize there. |
I think this PR got lost: is it still possible to land this? |
59e277c
to
f42c90a
Compare
@TrySound: Ready to merge! The |
It appear that a <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1 1" height="64" width="64">
<defs>
<symbol id="a" overflow="visible">
<path/>
</symbol>
</defs>
<use xlink:href="#a" style="fill:#000"/>
</svg>
@@@
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 1 1" height="64" width="64">
<defs>
<symbol id="a" overflow="visible">
<path/>
</symbol>
</defs>
<svg style="fill:#000">
<path/>
</svg>
</svg>
@@@
{"keepHref":true} And the resulting error:
|
…element (`svg` is the default). Add tests for `<symbol>` elements as target. Add comments.
Register dereferenceUses plugin correctly.
@TrySound: How can the parent node/element be retrieved from an And how can a |
parentNode is passed to element callback in visitor structuredClone can copy elements |
@TrySound: The tests all pass except for the regressions test, but not because of the plugin but of a failing test setup. A polyfill for |
Any updates on this? |
Also note that |
@kuhler-Stratege; @Pomax; @SethFalco: I merged all the changes and updated the plugin code and documentation. @SethFalco: How can I run the tests for non-preset default plugins as this one, or run tests only for a specific (this one) plugin? |
The CI tests still include Node @SethFalco: Is Node |
Note that Node 16 is no longer supported by Node itself, the current maintenance LTS is v18 and active LTS is v20 - it should definitely be removed from any CI tasks with |
All checks are passing except the One thing though: How can I force this plugin to run before/after specific other plugins, |
@TrySound is there anyone else who can be marked as reviewer? Sure, it's been years, but if the plugin still works, no reason to not still land it =) |
@SethFalco ☝🏻 |
This PR adds the
dereferenceUses
plugin, which de-references<use/>
elements, replacing it with the targeted node.It also takes the special attribute inheritance rules into account.
You can already try this feature branch by installing it using this
npm
command:npm install github:strarsis/svgo#dereferenceUses-plugin
Note that this plugin currently doesn't run by default and has to be explicitly enabled:
svgo --enable=dereferenceUses [...]
The original/source elements are kept by this plugin as those may still be needed,
but can (and will by default) be removed using the existing removeUselessDefs plugin (that runs afterwards by default).